-
Notifications
You must be signed in to change notification settings - Fork 315
Cleanup | Remove GenerateResourceStringsSource MSBuild target #3639
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Cleanup | Remove GenerateResourceStringsSource MSBuild target #3639
Conversation
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it!
This is actually used by Localization CI for generation of resource strings. Removing this target might break things. Would recommend taking a closer look for safety or adding necessary documentation around it to define its purpose. @paulmedynski @mdaigle fyi for ref: Link to usage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs careful observation into Localization workflow usage.
I also love these changes, especially since my IDE complains about ResourceNames not existing a lot. |
I don't think there's an easy way to validate this before the merge - you can carefully scan for the targets being removed and what its doing from the logs. or Fail Fast: Go ahead and merge, be diligent to observe the Loc CI and be ready to revert this change if Localization CI is not happy after few days. |
Can the Localization CI run against a branch other than main? I'm happy to resubmit a PR which merges to another branch if that'd help validate the behaviour. |
Description
At the moment, the build has a custom target,
GenerateResourceStringsSource
. This target has been around since the start of the repo; it runs prior to compile and generates a file which contains one const for each resource string. An example file isThis
ResourceNames
class is only used in theResCategory
andResDescription
attributes applied to certain properties within the implementation assemblies for netfx and netcore.This is redundant because the
ResCategoryAttribute
andResDescriptionAttribute
classes will already look up the parameter passed to them as a resource string. Switching to using this functionality (with associated test) means that we don't need theStringsHelper
class, which means that we don't need the target which generates it.Issues
None.
Testing
Two automated tests added to prove that the descriptions contain the localized values as we expect. One manual check by binding a PropertyGrid to a
SqlConnectionStringBuilder
(in netcore and netfx) to make sure that WinForms isn't performing something unusual to get the description in a way which bypasses the custom logic inResCategoryAttribute
andResDescriptionAttribute
.